fix(icons): fixed the various icon viewbox issues#565
Conversation
ArtBlue
requested review from
HenriqueLimas,
LuLaValva,
ianmcburnie and
saiponnada
|
There was a problem hiding this comment.
Pull request overview
Fixes 9 icons with incorrect viewBox attributes where viewBoxes were swapped, off-by-one, or wrong size. Also adds a viewBox validation check to the icon build script to prevent future regressions.
Changes:
- Fixed swapped viewBox/path data for
arrows-expandandfeedback-erroricon pairs (16↔24) - Corrected off-by-one and wrong-size viewBox values for
close-12,fingerprint-64,trend-down-16-fit,trend-up-16-fit, andspinner-20 - Added viewBox mismatch detection in
generate-images.tsbuild script
Reviewed changes
Copilot reviewed 3 out of 23 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/skin/src/svg/icon/*.svg (9 files) | Fixed viewBox attributes in source SVG files |
| packages/skin/src/svg/icons.svg | Updated combined icons SVG with corrected viewBoxes and paths |
| packages/skin/dist/svg/icons.svg | Regenerated dist combined icons SVG |
| packages/skin/dist/svg/icon/*.svg (9 files) | Regenerated dist individual icon SVGs |
| src/tags/master-icons.marko | Updated Marko template with corrected viewBoxes and paths |
| src/data/icons.json | Updated icon metadata (width/height) to match corrected viewBoxes |
| packages/skin/scripts/generate-images.ts | Added viewBox validation during icon build |
saiponnada
left a comment
saiponnada
left a comment
There was a problem hiding this comment.
There might be issue with this approach of adjusting the viewbox like this. The core problem here is viewBox dimensions were changed to match filenames, but the path data inside those SVGs wasn't redrawn to fit the new coordinate space.
So when you change the viewBox without updating the paths, the icon renders at the wrong size, position, or with unintended whitespace.
For example I noticed spinner-20 the circle was off and not aligned center properly.
Screen.Recording.2026-03-18.at.11.26.55.AM.mov
There was a problem hiding this comment.
I can see the X icon is not centered appropriately here.
There was a problem hiding this comment.
These trend icons need to be recentered.
|
Closed this PR. Opening another one with the modest icon issue fixes. |
Description
Fixed 9 icons with incorrect viewBox attributes in their source SVG files. All changes have been verified with the automated validation script.
Files Fixed
Group 1: Swapped ViewBoxes (4 files)
Arrows Expand Icons:
icon-arrows-expand-16.svg- Changed from24x24to16x16icon-arrows-expand-24.svg- Changed from16x16to24x24Feedback Error Icons:
icon-feedback-error-16.svg- Changed from24x24to16x16icon-feedback-error-24.svg- Changed from16x16to24x24Group 2: Off-by-One Errors (4 files) -⚠️ NEED TO VERIFY WITH DESIGN
icon-close-12.svg- Changed from13x12to12x12icon-fingerprint-64.svg- Changed from65x64to64x64icon-trend-down-16-fit.svg- Changed from13x16to16x16icon-trend-up-16-fit.svg- Changed from13x16to16x16Group 3: Wrong Size Used (1 file) -⚠️ NEED TO VERIFY WITH DESIGN
icon-spinner-20.svg- Changed from16x16to20x20Validation Script Updates
Added automated viewBox validation to
packages/skin/scripts/generate-images.ts:What it does:
-16,-24)npm run build:iconsExclusions (intentional non-square icons):
-coloredsuffix (payment methods with specific aspect ratios)-logoin filename (brand marks)icon-psa(PSA brand-specific dimensions)Example output:
npm run build:icons # ✅ No viewBox mismatches detected!Impact
These fixes ensure:
Testing
Run the build script to verify:
cd packages/skin npm run build:iconsThe script will flag any new viewBox mismatches going forward.
Checklist